🐛 Declare __tablename__ on the metaclass (improve pyright compatibility)#1821
Open
estarfoo wants to merge 1 commit into
Open
🐛 Declare __tablename__ on the metaclass (improve pyright compatibility)#1821estarfoo wants to merge 1 commit into
__tablename__ on the metaclass (improve pyright compatibility)#1821estarfoo wants to merge 1 commit into
Conversation
Member
Author
|
@YuriiMotov, thank you very much for the review! Sorry for being slow to get back to this. I agree that the dynamic table name is a valid use case and deserves support along with the static case. However:
from sqlalchemy.orm.decl_api import _declared_directive
# …
__tablename__: ClassVar[str | Callable[..., str] | _declared_directive[str]]
__tablename__: ClassVar[Any]But the second option loses typing for table names. Any preference, or do you maybe see another way? |
The `SQLModel` base declared `__tablename__` both as `ClassVar[str | Callable[..., str]]` and as a `@declared_attr` method. Type checkers (pyright) see the descriptor type from `@declared_attr`, so `__tablename__ = "my_table"` in a subclass is rejected as a type mismatch even though it works at runtime; and the `ClassVar` on the base both leaks into the constructor signature and conflicts with a descriptor override such as `@declared_attr.directive`. Move the declaration to `SQLModelMetaclass` as `__tablename__: str`, setting the default in `SQLModelMetaclass.__new__` (in the class dict, before class creation) unless the user supplied one. Because the attribute now lives on the metaclass: - explicit names (`__tablename__ = "my_table"`) narrow to `str` rather than the `str | Callable` union, so reads are usable without casts; - it is not collected as a model field, so it never appears in the constructor; - a dynamically computed name via `@declared_attr.directive` type-checks in pyright basic mode, leaving only a single reportIncompatibleVariableOverride in standard/strict mode, inherent to overriding a class attribute with a descriptor. Tests added for default name, explicit override, inheritance, non-table models, and the `@declared_attr.directive` form. Fixes fastapi#98. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
24d66f7 to
415c8fc
Compare
__tablename__ default from @declared_attr to metaclass__tablename__ on the metaclass (improve pyright compatibility)
Author
|
@YuriiMotov, following up on dynamic table names: neither option from above turned out to be satisfying, so here's a rework shifting the The current version would still produce a pyright message. I don't see a way around this at the moment, and I think it's an intrinsic limitation coming from SQLAlchemy. I'm open to suggestions on this, though! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Move
__tablename__from a@declared_attrmethod onSQLModelto a declaration onSQLModelMetaclass, with the default set inSQLModelMetaclass.__new__before class creation (unless the user supplied one).This resolves the type-level contradiction between the
ClassVar[str | Callable]declaration and the@declared_attrdescriptor, letting__tablename__ = "my_table"work without# type: ignorefor pyright.Caveat: dynamically computed names via
@declared_attr.directive(per @YuriiMotov's review below) type-check in pyright basic mode. In standard/strict mode, onereportIncompatibleVariableOverridediagnostic remains.Tests added for default name, explicit override, inheritance, non-table models, and the
@declared_attr.directiveform.Fixes #98.
This is split out from #1820, dropping those changes which would be resolved by #1345 or #1806.
(Description edited following source branch update from 24d66f7 to 415c8fc.)